feat(amplifier): durable session tracking via events.jsonl (two-lane tracker, locator, ledger)#514
Conversation
Final plan for durable amplifier session tracking, including Phase 0 empirical findings and the outcomes of adversarial review rounds. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…tracker, locator, ledger) Replace output-silence heuristics with the amplifier events.jsonl lifecycle as the source of truth for turn tracking: - events.jsonl lifecycle drives turn state: prompt:submit / prompt:complete are the turn boundaries; the deadman timer is repurposed as a force-read failsafe and never fabricates completions in the events lane. - PTY <-> session association via Enter <-> new-session-dir correlation with session:config cwd confirmation. New locator + controller flow through the shared bindSession/broadcast paths; the coordinator slow-path is preserved. - Catch-up state-sync prevents historical turn replay on attach. - Claude fast path widened to cover amplifier. - TurnCompletionLedger extracted from 4 duplicated trackers (claude/codex/opencode/amplifier). - claude/amplifier wiring unified via a shared activity-wiring factory. - Feature flag FRESHELL_AMPLIFIER_EVENTS_TRACKING, default-on; the degraded lane is the prior behavior verbatim. - Hardening from a 3-agent adversarial review plus red-team re-verification: watcher-root containment, serialized attach, tailer memory caps, pre-Enter epsilon + stat-anchored rescan, and a grace force-read backstop. WS protocol and client are untouched. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…contradiction - Provider test: session dirs containing only events.jsonl (kill -9 before first prompt:complete, finding E6) are excluded from listSessionFiles() - Plan doc: remove amplifier-activity-wiring.ts from the Untouched list (superseded by Phase 4 wiring unification) Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62dd6b5b14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (catchUp && (effect.kind === 'turn.began' || effect.kind === 'turn.completed')) { | ||
| if (effect.kind === 'turn.began') attachment.catchUpBeganAt = effect.at | ||
| continue |
There was a problem hiding this comment.
Preserve fast first-turn completions during catch-up
When the locator binds a fresh Amplifier session, it waits for the correlation window to close before calling attachTailer(..., 'start'); if the first turn completes inside that window, the degraded tracker is already busy but its idle debounce has not fired yet. This catch-up branch suppresses the prompt:complete record, and the later catch-up adoption only handles a final busy reducer state, while enableEventsLane has already cleared the degraded debounce timer. In that scenario the terminal silently reverts later via submit-grace with no terminal.turn.complete/chime for the completed turn, so catch-up needs to complete an already-busy tracker when the catch-up reducer ends idle rather than treating that completion as historical.
Useful? React with 👍 / 👎.
| armed.ready = (async () => { | ||
| armed.cwdNormalized = await normalizeRealCwd(record.cwd!) | ||
| armed.snapshot = await this.snapshotSessionDirs() | ||
| })().catch(() => {}) | ||
| this.armed.set(record.terminalId, armed) |
There was a problem hiding this comment.
Capture session snapshot before accepting input
The terminal is added to armed before the async pre-spawn snapshot finishes, so an immediate user submit can create the Amplifier session directory before snapshotSessionDirs() resolves. That real session dir is then recorded in armed.snapshot, and tryResolveWindow later excludes snapshot dirs from eligibility, causing the locator to miss the in-flight session until the metadata fallback after completion or never if the process is killed first. The snapshot should be taken before the terminal can open a correlation window, or dirs born after spawnedAt should not be excluded as pre-existing.
Useful? React with 👍 / 👎.
…ingle events-driven path Maintainer decision: one code path only. - Delete FRESHELL_AMPLIFIER_EVENTS_TRACKING; locator, controller, integration, and the amplifier fast path are unconditional. - Delete the degraded lane (output-silence debounce, first-output gate, deadman completion fabrication) and its tests, including the pre-existing timing-heuristic pins. - Signal loss (tailer degrade/detach, or no events.jsonl because the bundle lacks the hooks-logging module) now means idle-and-stop: phase reverts to idle with no turn.complete, one structured warn; only submit-grace provisional-busy pulses remain for such terminals. - Docs updated (plan doc §6/§7/Appendix A, ACTIVITY_TRACKING_SPEC). Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…port) Read-only investigation spec: root-cause + implementer-ready plan for amplifier PTY panes restoring as blank sessions after a server restart. Reference #514 (05c6b1f) adds the server-side AmplifierSessionLocator that associates a running amplifier PTY with its session id; the rust port never ports that association or emits terminal.session.associated, so the client captures no sessionRef to persist. Minimal fix is server-only (port the locator + one association broadcast); resume-arg construction and the provider-generic client restore path (incl. #516 cherry-pick) already exist. No shared/ or src/ changes expected. Generated with Amplifier Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Ports #514's AmplifierSessionLocator + association-broadcast mechanism to the Rust server, per docs/plans/2026-07-18-amplifier-restore-spec.md. An amplifier terminal pane running when the server restarted was coming back as a blank new session (no resume) because the port had no live PTY<-> session association for any provider (identity.rs never associated a running terminal with a session id, so terminal.session.associated was never emitted for amplifier). Slice A (crates/freshell-sessions/src/amplifier_locator.rs): a poll-driven AmplifierLocator correlates a fresh amplifier PTY's first Enter/submit with the new ~/.amplifier/projects/.../sessions/<id>/ dir amplifier lazily creates, confirmed via that dir's events.jsonl session:start + cwd-bearing session:config. Mirrors amplifier-session-locator.ts's semantics: two cwd-confirmed candidates in one correlation window -> refuse + log (never guess); zero -> keep watching; subagent/underscore dirs never candidates. Uses polling instead of a live filesystem watcher (module doc explains why this is a faithful substitute), seeding its known-dirs baseline eagerly at construction rather than on the first poll tick, closing a startup race where a session dir created between construction and that first tick would otherwise be wrongly treated as pre-existing. Slice B (crates/freshell-ws/src/amplifier_association.rs): the input-submit seam (armed amplifier terminals only) feeds the locator from the WS terminal.input path; on a Located resolution, binds the terminal's identity and broadcasts terminal.session.associated + terminal.meta.updated -- the same wire messages the frozen client's generic reconcileTerminalSessionAssociation + restore machinery already handle for every other provider. No client or shared/ changes were needed or made; resume-arg derivation (amplifier resume <id>) was already generic (terminal.rs:744-753, cli_launch_goldens.rs G-A2). Tests: 13 locator unit tests (fresh/foreign/subagent/ambiguous/zero- candidate/resume-skip/watch-path/lazy-projects-dir/disarm) + 6 association/broadcast tests, all in crates/freshell-sessions and crates/freshell-ws. One new Playwright e2e scenario (test/e2e-browser/specs/amplifier-restore-rust.spec.ts) with a fake amplifier CLI fixture (test/e2e-browser/fixtures/fake-amplifier-cli.mjs) proves the pane restores via `amplifier resume <id>` after a real server restart, and a never-submitted sibling pane restores fresh with no false bind. Registered rust-chromium-only in playwright.config.ts: the frozen legacy server/ predates upstream #514 and has no amplifier provider at all, so this is an absent feature there, not a parity gap. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…or sweep; fix e2e cli-detection rationale Two review fix-forwards on the amplifier-restore feature (008e8cf): 1. AmplifierLocator::tick() unconditionally walked <amplifier_home>/projects/*/sessions/* every 150ms forever, using blocking std::fs directly on a tokio worker thread, even with zero armed terminals. #514's reference only watches while >=1 armed terminal exists. - tick() now short-circuits with ZERO filesystem I/O whenever `armed` is empty (module doc explains the invariant). - arm() re-baselines `known_dirs` (extend, never shrink) on the idle->armed transition, from the SAME fresh disk read already taken for `armed.snapshot`. This is belt-and-suspenders -- binding correctness never depended on known_dirs freshness (armed.snapshot, taken live at every arm() call regardless of sweep cadence, already excludes every pre-existing dir) -- the refresh's real payoff is avoiding wasted events.jsonl probes of a potentially large accumulated-while-idle set. - drain_and_associate is now async and runs the locator's blocking tick() inside tokio::task::spawn_blocking, mirroring SessionIndex::snapshot's identical wrapping for the analogous spawn_sessions_sweep poll. New tests (RED verified before the fix, GREEN after): tick_while_unarmed_performs_zero_filesystem_scans (via a new fs_scan_count test/diagnostic hook, mirroring armed_count's convention) and dir_created_while_idle_never_binds_but_post_arm_dir_still_locates. 2. amplifier-restore-rust.spec.ts's setAvailableClis Redux-dispatch workaround was justified by a comment claiming CLI detection uses a hardcoded 5-name set excluding amplifier. Verified false: detection is derived from genuinely discovered extension manifests (ExtensionRegistry::cli_detection_specs); DEFAULT_CLI_DETECTION_SPECS is dead reference-parity code (extensions.rs's own doc comment says so, and cargo warns unused). extensions/amplifier/freshell.json is a real `category: "cli"` manifest, and RustServer spawns with cwd: PROJECT_ROOT -- exactly where the cwd-relative extensions/ scan looks. Removed the workaround and ran the spec: it passes via genuine server-side detection with no client-side dispatch needed. The codingCli.enabledProviders seeding in setupHome stays -- a fresh home has no persisted knownProviders, so SettingsStore::load's seed-when-missing branch does NOT auto-enable newly discovered providers (only the append-to-existing branch does); explicit enabling is genuinely required, matching the existing FreshCodex restore-matrix precedent this comment already cited. Verification: cargo test -p freshell-sessions -p freshell-ws all green (77 unit + integration tests, no regressions); fmt/clippy clean on touched files; frozen server/shared/src/ untouched; cargo build --release -p freshell-server; amplifier-restore-rust e2e green 2x; restore-matrix green once, rust-chromium. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Implementer-ready spec bringing opencode TERMINAL panes to durable-restore parity with codex and amplifier. Legacy has no opencode terminal locator (beyond-legacy); designed per the amplifier-locator precedent (#514). - opencode 1.18.3 stores sessions in a single SQLite opencode.db (WAL), not one dir/file per session -> correlation is a session-table row-diff, not a dir-appearance watch + events.jsonl probe. - Recommends (b) a sibling opencode_locator.rs (+ opencode_association.rs) over (a) a provider-parameterized locator: detection substrates share zero code and the opencode variant is strictly simpler (no probe FSM). - Resume syntax opencode --session <id> already wired via cli_launch; client generic association machinery already opencode-ready. Only the server-side locator + terminal.session.associated broadcast is missing. Read-only investigation; no code changed. Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ite row-diff Brings opencode CLI terminal panes to durable-restore parity with codex/amplifier (survive tab closes + server restarts), per docs/plans/2026-07-18-opencode-terminal-restore-spec.md. Legacy has NO opencode terminal<->session association anywhere (spec §2); this is a beyond-legacy capability designed by analogy to the amplifier-locator precedent (#514), NOT a port of existing behavior. - crates/freshell-sessions/src/opencode_locator.rs (new): sibling correlation core to amplifier_locator, adapted for opencode's SQLite session-row-diff substrate (vs. amplifier's directory-appearance + events.jsonl probe). Arms on a fresh opencode terminal create, polls a bounded `list_sessions_since` query, and resolves via a window whose lower bound is always `arm_ms - PRE_EPSILON_MS` and whose upper bound is spawn-anchored (no Enter yet) or Enter-anchored (extends outward once note_submit fires) -- robust to opencode writing its session row either at spawn or lazily at first prompt (spec §4.4 open question). No probe-retry FSM needed: a session row carries every reject-rule field (parent_id, time_archived, 3-views marker, directory, time_created) synchronously, unlike amplifier's events.jsonl which lands over several ticks. Idle short-circuit (zero DB reads while unarmed) and per-terminal arm-time known-ids snapshot mirror amplifier_locator's f53196a fix. - crates/freshell-sessions/src/parse/opencode.rs: adds `list_sessions_since`/`run_opencode_candidate_query`, a bounded (`time_created >= floor LIMIT n`) sibling to the existing unbounded `list_sessions` query, so a poll tick never scans the full (potentially multi-GB, WAL-mode) `session` table. - crates/freshell-ws/src/opencode_association.rs (new): WS controller mirroring amplifier_association.rs -- arm/disarm at terminal lifecycle points, feed submit-shaped input, drain+bind+broadcast `terminal.session.associated` + `terminal.meta.updated` on location. - crates/freshell-ws/src/terminal.rs, lib.rs; crates/freshell-server/ src/main.rs: sibling wiring alongside every existing amplifier_locator touch point (WsState field, input-submit seam, create-time arm, exit-time disarm, sweep construction) -- zero client changes needed (spec §1/§3.4: isDurableProviderSessionId already accepts `ses_*`, the generic reconcile/reopen path already handles any provider). - test/e2e-browser/fixtures/fake-opencode-terminal.mjs (new) + specs/opencode-terminal-restore-rust.spec.ts (new, rust-chromium only, KNOWN DIVERGENCE documented): fake opencode CLI writes a real session row into a temp opencode.db on first Enter; proves association -> server restart -> `opencode --session <id>` respawn, alongside a never-submitted pane restoring fresh. Row-timing note: the spawn-anchored window shape is proven deterministically by opencode_locator's own unit tests (row_created_at_spawn_before_any_enter_resolves_via_spawn_window), which control row-vs-arm timing to the millisecond; the e2e fixture models the Enter-anchored shape for a clean, symmetric negative control (mirrors fake-amplifier-cli.mjs). Tests: 17 opencode_locator + 6 opencode_association unit tests (all RED-then-GREEN verified), plus the existing 13 amplifier_locator + 6 amplifier_association tests unweakened. New e2e green 2x; existing amplifier-restore-rust e2e green 1x (unweakened). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…1..06 Add a new 'E2E-discovered intentional divergences' section to port/oracle/DEVIATIONS.md, kept deliberately separate from the /council-adjudicated DEV-NNNN oracle fingerprint whitelist (which is scoped to the mechanical harness). The EDEV-xx entries record intentional old-vs-new divergences surfaced by the Playwright legacy-vs-rust browser matrix, each with: what differs, why intentional (better side + decider), evidence (spec::test + commit), and one plain-English user-impact line. Entries (each verified against its cited spec + commit before writing): - EDEV-01 WS Origin policy enforced (close 4011) vs legacy advisory-only; includes the origin.rs allowlist mechanism for operators (ALLOWED_ORIGINS replaces defaults, EXTRA_ALLOWED_ORIGINS appends, same-origin always ok, null always rejected). safe03-origin-matrix.spec.ts, f18554a. - EDEV-02 scrollback search bounded to the retained replay window vs legacy's unbounded this.lines. term13-scrollback-boundary.spec.ts, fc1fc3f. - EDEV-03 settings-save write failures surfaced as 500 + envelope, no in-memory commit. cfg03-backup-restore.spec.ts GAP2, 8c78e48. - EDEV-04 config auto-restore + forensic copy + config.fallback notice vs legacy default-overwrite + lost backup. cfg03-backup-restore.spec.ts, 41b0414 + 8c78e48. - EDEV-05 claude interrupt missing-session error text static vs session-id-embedded (cosmetic). 57a8281. - EDEV-06 whitespace-only auth token rejected at startup (hardening), discovered via the KNOWN DIVERGENCE sweep. safe01-auth-matrix.spec.ts, f18554a. Sweep of 'KNOWN DIVERGENCE' across test/e2e-browser/specs adjudicated the remaining hits as skip: amplifier-restore / opencode-terminal-restore / sidebar-click-resume / session-directory-matrix are frozen-legacy version artifacts (predate upstream #514 05c6b1f), and agent-checkpoint-rewind is a client-side test-timing flake (routes proven by 41 rust tests) -- none are intentional behavioral divergences. Also resolve the open DEVIATIONS scope question in the campaign status doc. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…poisoning repro spec boot.rs::logs_client validated POST /api/logs/client bodies via client_logs_issues and returned 204 on success, but never emitted the validated entries anywhere -- browser-reported errors/warnings vanished silently. Confirmed during a real incident where the client's own error reports were needed but absent from the log. Add emit_client_log_entries(): after validation passes, emit one tracing event per entry at ITS OWN severity (error/warn/info/debug), target 'freshell_server::client_logs', mirroring server/client-logs.ts:33-53's `log[level](...)` parity (message = entry.message || entry.event || 'Client log'). Runs inside the request's http_request span, so request_id/route/method are already merged onto every event by JsonLayer. Entries are already bounded to 1..=200 by the existing zod- parity validation, so no separate cap is needed. RED test added first (crates/freshell-server/tests/ diag02_client_log_emission.rs): spawns the real binary, POSTs a 4-entry batch (one per severity), and asserts each lands in rust-server.jsonl at the right level/target/message. Failed before the fix (no entries emitted); passes after. Also add test/e2e-browser/specs/rest-tab-persistence.spec.ts: acceptance evidence for a tab-poisoning incident. A REST-created tab with mode:'amplifier' (a real, extension-manifest-registered launch target on both servers) survives untouched in localStorage's freshell.layout.v3 after a reload, but the tab strip renders empty -- because zPersistedLayoutPayload.safeParse (src/store/persistedState.ts) embeds zTabMode, a hardcoded enum missing 'amplifier', and one bad tab fails the WHOLE combined-layout parse, not just that tab. Client-side, shared bug; authored test.fail()-annotated (documented flip instruction in the file) so the suite stays green until the upstream client fix lands, at which point the unexpected pass trips a hard failure as the signal to flip it to a normal assertion. KNOWN DIVERGENCE: gated rust-chromium-only in playwright.config.ts. This branch's frozen server/ tree predates origin/main 05c6b1f (#514, which added amplifier support) -- verified via `git merge-base --is-ancestor` returning false, and matches the identical divergence note already established for amplifier-restore-rust.spec.ts / session-directory- matrix.spec.ts. Legacy cannot create the poisoned tab via REST on this branch; the underlying client bug is shared/frozen code and would affect legacy equally through any other writer of the layout key. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Problem
Amplifier was the only provider with no durable signals:
turn.completeevents.Approach
Per docs/plans/2026-07-08-amplifier-session-durability-plan.md, which includes settled Phase 0 empirical findings from live CLI experiments:
events.jsonl(amplifier.log1.0.0) drives turn lifecycle —prompt:submit/prompt:completeare the turn boundaries.session:configcwd confirmation, flowing through the sharedbindSession/broadcast paths; claude fast-path widened to amplifier.DRY
TurnCompletionLedgerextracted from 4 byte-identical tracker duplications.shared/ws-protocol.ts,ws-handler.ts,session-association-coordinator.ts, and all client code untouched.Hardening
Verification
Upstream issues filed
Upstream asks for
microsoft/amplifier-app-cli(plan §12): microsoft/amplifier#314, microsoft/amplifier#315, microsoft/amplifier#316, microsoft/amplifier#317, microsoft/amplifier#318Generated with Amplifier